Skip to content

Conversation

@asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Dec 20, 2025

User description

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement


Description

  • Replace reflection-based exception construction with method references

  • Add generic type parameter to W3CError for type safety

  • Use Function<String, T> for exception constructors instead of Class

  • Remove java.lang.reflect.Constructor import and add java.util.function.Function


Diagram Walkthrough

flowchart LR
  A["Reflection-based<br/>Constructor lookup"] -->|"Replace with"| B["Method references<br/>Exception::new"]
  C["Class parameter<br/>in W3CError"] -->|"Change to"| D["Function parameter<br/>for construction"]
  E["Try-catch<br/>ReflectiveOperationException"] -->|"Simplify to"| F["Direct function<br/>application"]
Loading

File Walkthrough

Relevant files
Enhancement
ErrorCodec.java
Refactor exception construction from reflection to method references

java/src/org/openqa/selenium/remote/ErrorCodec.java

  • Removed java.lang.reflect.Constructor import and added
    java.util.function.Function import
  • Made W3CError class generic with type parameter
    WebDriverException>
  • Changed W3CError constructor to accept Function instead of
    Class
  • Replaced reflection-based exception instantiation in decode() method
    with direct function application
  • Updated all 28 error definitions to use method references (e.g.,
    ScriptTimeoutException::new) instead of class literals
  • Changed field visibility from public to private in W3CError class
  • Added @SafeVarargs and @SuppressWarnings annotations to handle generic
    type reification
+49/-48 

@asolntsev asolntsev added this to the 4.40.0 milestone Dec 20, 2025
@asolntsev asolntsev self-assigned this Dec 20, 2025
@selenium-ci selenium-ci added the C-java Java Bindings label Dec 20, 2025
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unchecked type inference: The new W3CError type-reification via varargs and unchecked cast may mis-identify
exception at runtime (and decode no longer wraps constructor failures), which could lead
to unexpected exceptions without contextual handling.

Referred Code
  return w3CError.exceptionConstructor.apply(message);
}

private W3CError<? extends WebDriverException> fromThrowable(Throwable throwable) {
  return ERRORS.stream()
      .filter(err -> err.exception.isAssignableFrom(throwable.getClass()))
      .findFirst()
      .orElse(DEFAULT_ERROR);
}

private static class W3CError<T extends WebDriverException> {

  private final String w3cErrorString;
  private final Function<String, T> exceptionConstructor;
  private final Class<T> exception;
  private final int httpErrorCode;

  @SafeVarargs
  @SuppressWarnings("unchecked")
  public W3CError(
      String w3cErrorString,


 ... (clipped 7 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Simplify constructor by passing class directly

Simplify the W3CError constructor by directly passing the exception's Class
object instead of using a complex varargs parameter to infer the type at
runtime. This change makes the code more robust and easier to understand.

java/src/org/openqa/selenium/remote/ErrorCodec.java [181-192]

-@SafeVarargs
-@SuppressWarnings("unchecked")
 public W3CError(
     String w3cErrorString,
     Function<String, T> exceptionConstructor,
-    int httpErrorCode,
-    T... reified) {
+    Class<T> exception,
+    int httpErrorCode) {
   this.w3cErrorString = w3cErrorString;
   this.exceptionConstructor = exceptionConstructor;
-  this.exception = (Class<T>) reified.getClass().getComponentType();
+  this.exception = exception;
   this.httpErrorCode = httpErrorCode;
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This is a valuable suggestion that improves the code's robustness and readability by replacing a complex type-reification workaround with a more direct and explicit approach.

Medium
Possible issue
Null-safe exception message

In the decode method, prevent passing a null message to exception constructors
by providing a default empty string if the message is null.

java/src/org/openqa/selenium/remote/ErrorCodec.java [164]

-return w3CError.exceptionConstructor.apply(message);
+return w3CError.exceptionConstructor.apply(message != null ? message : "");
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential NullPointerException if the response message is null and provides a simple fix, improving the robustness of the error decoding logic.

Low
  • More

@asolntsev asolntsev merged commit 130c11d into SeleniumHQ:trunk Dec 20, 2025
41 checks passed
@asolntsev asolntsev deleted the refactor/error-codec branch December 20, 2025 14:46
@asolntsev asolntsev added E-easy An easy issue to implement or PR to review I-enhancement Something could be better labels Dec 20, 2025
This was referenced Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings E-easy An easy issue to implement or PR to review I-enhancement Something could be better Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants